fix(backtesting): annualize bar-count equity curves by trading days (#12)#37
Open
bradsmithmba wants to merge 1 commit into
Open
Conversation
annualized_return divided the period by 365.25 in both branches of calculate_all_metrics. That is correct when the equity curve has a datetime column (elapsed calendar days / calendar days per year), but wrong when it does not: there each row is a trading bar, and a trading-bar count divided by 365.25 over-annualizes the return (e.g. 252 bars at +20% reported ~30% instead of ~20%). Use self.trading_days to annualize the no-datetime (bar-count) branch; leave the calendar-days branch unchanged. trading_days was already honored by the Sharpe/Sortino/volatility calculations — this aligns annualized_return with it for bar-indexed curves. Verified: 252 bars at +20% total now yields 0.20 annualized (was ~0.30). Adds a regression test for the bar-count branch. Closes cloudtrainerwork#12 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PerformanceCalculator.calculate_all_metrics()annualized returns by dividing the period by365.25in both branches:datetimecolumn,daysis elapsed calendar days, so/365.25is correct.daysis a count of trading bars, and dividing that by365.25over-annualizes the return.Example: 252 trading bars at +20% total reported ~30% annualized instead of ~20%.
Closes #12.
Fix
Annualize the no-datetime branch by
self.trading_days; leave the calendar branch unchanged.trading_dayswas already used by the Sharpe/Sortino/volatility calculations; this makesannualized_returnconsistent with it for bar-indexed curves. (The issue's "trading_days is never used" note was slightly off — it was used everywhere except this annualization.)Verification
Adds
test_annualized_return_without_datetime_uses_trading_days.Scope note
tests/backtesting/test_performance.pyhas 3 pre-existing failures unrelated to this change —test_win_rate_calculation,test_sharpe_ratio_calculation, andtest_annualizationall assert incorrect expected values (wrong oracles). They use the calendar branch this PR does not touch and are tracked by #14. My new test passes; those remain for #14.🤖 Generated with Claude Code